-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refector: remove pause/result from provider interface #329
Conversation
} | ||
|
||
// Stop will stop a paused or running microvm. | ||
// Stop will stop a running microvm. | ||
func (p *fcProvider) Stop(ctx context.Context, id string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we using stop anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No we are not. I did also consider deleting it but decided to leave and just make it do the same as delete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we not just remove it if it serves no purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not. I did make me wonder if we should be doing different things in stop and delete. For example, in Stop
should we stop the firecracker process and then in Delete
we delete the files. But lets create a ticket for follow up and remove stop for the time being. wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added #330
As we are not supporting pausing / resuming microvms at this stage these functions have been removed from the microvm provider interface. The firecracker implementation has these removed as well and any tests have been updated. Signed-off-by: Richard Case <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #329 +/- ##
==========================================
+ Coverage 58.03% 58.04% +0.01%
==========================================
Files 51 51
Lines 2509 2510 +1
==========================================
+ Hits 1456 1457 +1
- Misses 936 938 +2
+ Partials 117 115 -2
Continue to review full report at Codecov.
|
What this PR does / why we need it:
As we are not supporting pausing / resuming microvms at this stage these
functions have been removed from the microvm provider interface. The
firecracker implementation has these removed as well and any tests have
been updated.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
Checklist: